Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

report_clear() efficiency, plus clear after grep #649

Merged
merged 1 commit into from
Jul 6, 2017

Conversation

rolandwalker
Copy link
Contributor

2b384f6 introduced a performance regression due to use of wclear(), which dirties the entire screen. This can be seen in the form of flickering: enter the main view and hold down <Up>.

The core issue being addressed in 2b384f6 is that readline can leave an incorrect cursor position in virtual window newscr (which can be inspected using getyx()).

So, explicitly reset the newscr cursor position post-readline, which enables switching to the much more efficient werase() for erasing the status area.

Incidentally fix the missed case of report_clear() after a grep prompt.

2b384f6 introduced a performance regression due to use of wclear(),
which dirties the entire screen.  The core issue being addressed in
2b384f6 is that readline can leave an incorrect cursor position in
virtual window newscr.

So, explicitly reset the newscr cursor position post-readline, which
enables switching to the much more efficient werase() for erasing the
status area.

Incidentally fix the missed case of report_clear() after a grep prompt.
jonas added a commit that referenced this pull request Jul 6, 2017
Follow-up to aa84d85 to fix performance
regression noted in #649. Also cleans up open_prompt to not clear the
status window.
jonas added a commit that referenced this pull request Jul 6, 2017
Follow-up to aa84d85 to fix performance
regression noted in #649. Also cleans up open_prompt to not clear the
status window.
@jonas
Copy link
Owner

jonas commented Jul 6, 2017

2b384f6 was reverted and replaced with the report_clear code in aa84d85. I've made a follow up in #650 if you have time to review. The other stuff looks good.

@rolandwalker
Copy link
Contributor Author

Sure, 2b384f6 onward. aa84d85 has the same issue with calling wclear() frequently.

@rolandwalker
Copy link
Contributor Author

To clarify something I should have explained in the original comment here, set_cursor_pos() … werase() … doupdate() is a surgical strike that minimizes redrawing.

@jonas
Copy link
Owner

jonas commented Jul 6, 2017

Thanks for clarifying. I like the use of werase instead of wclear.

@jonas jonas merged commit fdf3cd7 into jonas:master Jul 6, 2017
@rolandwalker rolandwalker deleted the clear-status-by-werase branch July 6, 2017 22:06
@jonas jonas modified the milestone: tig-2.3 Jul 8, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants